Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix check on subclass for typing.Union in _infer_multiple_outputs for Python 3.10+ #36728

Merged
merged 2 commits into from
Jan 11, 2024
Merged

Fix check on subclass for typing.Union in _infer_multiple_outputs for Python 3.10+ #36728

merged 2 commits into from
Jan 11, 2024

Conversation

Taragolis
Copy link
Contributor

#36652 introduced bug for Python 3.10+ when task decorated function has annotation as object and not as class

This also fix broken main for Python 3.10+ tests:

  • tests/always/test_example_dags.py::test_should_be_importable[tests/system/providers/google/cloud/cloud_sql/example_cloud_sql_query_postgres.py]
  • tests/always/test_example_dags.py::test_should_be_importable[tests/system/providers/google/cloud/cloud_sql/example_cloud_sql_query_mysql.py]

airflow/tests/system/providers/google/cloud/cloud_sql/example_cloud_sql_query_postgres.py

Lines 197 to 198 in 59b32dc

@task
def get_public_ip() -> str | None: 

Traceback (most recent call last):
  File "/Users/taragolis/Projects/common/airflow/tests/system/providers/google/cloud/cloud_sql/example_cloud_sql_query_postgres.py", line 198, in <module>
    def get_public_ip() -> str | None:
  File "/Users/taragolis/Projects/common/airflow/airflow/decorators/python.py", line 75, in python_task
    return task_decorator_factory(
  File "/Users/taragolis/Projects/common/airflow/airflow/decorators/base.py", line 646, in task_decorator_factory
    decorator = _TaskDecorator(
  File "<attrs generated init airflow.decorators.base._TaskDecorator>", line 8, in __init__
    _setattr('multiple_outputs', __attr_factory_multiple_outputs(self))
  File "/Users/taragolis/Projects/common/airflow/airflow/decorators/base.py", line 353, in _infer_multiple_outputs
    return issubclass(ttype, Mapping)
  File "/Users/taragolis/.pyenv/versions/3.10.1/lib/python3.10/typing.py", line 1138, in __subclasscheck__
    return issubclass(cls, self.__origin__)
  File "/Users/taragolis/.pyenv/versions/3.10.1/lib/python3.10/abc.py", line 123, in __subclasscheck__
    return _abc_subclasscheck(cls, subclass)
TypeError: issubclass() arg 1 must be a class

^ Add meaningful description above
Read the Pull Request Guidelines for more information.
In case of fundamental code changes, an Airflow Improvement Proposal (AIP) is needed.
In case of a new dependency, check compliance with the ASF 3rd Party License Policy.
In case of backwards incompatible changes please leave a note in a newsfragment file, named {pr_number}.significant.rst or {issue_number}.significant.rst, in newsfragments.

@Taragolis Taragolis mentioned this pull request Jan 11, 2024
Merged
@Taragolis Taragolis added the full tests needed We need to run full set of tests for this PR to merge label Jan 11, 2024
@Taragolis Taragolis closed this Jan 11, 2024
@Taragolis Taragolis reopened this Jan 11, 2024
@eladkal eladkal added this to the Airflow 2.8.1 milestone Jan 11, 2024
@eladkal eladkal added the type:bug-fix Changelog: Bug Fixes label Jan 11, 2024
@Taragolis
Copy link
Contributor Author

Taragolis commented Jan 11, 2024 •
edited
Loading

During fix one problem another happen 🙄 🙄 🙄

FAILED tests/models/test_mappedoperator.py::test_mapped_render_template_fields_validating_operator - TypeError: Can't compile non template nodes
FAILED tests/models/test_mappedoperator.py::test_mapped_expand_kwargs_render_template_fields_validating_operator - TypeError: Can't compile non template nodes

However this fixes or othe merged PR should not be a reason for that, and it happen (I guess) only with never version of Jinja2==3.1.3 which are released for about 7 hours ago

< Jinja2==3.1.2
---
> Jinja2==3.1.3

@Taragolis Taragolis mentioned this pull request Jan 11, 2024
Merged
@Taragolis
Copy link
Contributor Author

Everything looks green except one non-relevant failure. Merge

@Taragolis Taragolis merged commit f1d8297 into apache:main Jan 11, 2024
77 of 78 checks passed
@Taragolis Taragolis deleted the fix-union-type branch January 11, 2024 11:12
potiuk pushed a commit that referenced this pull request Jan 13, 2024
@Taragolis @potiuk
Fix check on subclass for typing.Union in _infer_multiple_outputs…
144eb51 
… for Python 3.10+ (#36728)

* Fix check on subclass for `typing.Union` in `_infer_multiple_outputs` for Python 3.10+

* Limit PEP 604 test by Python 3.10

(cherry picked from commit f1d8297)
ephraimbuddy pushed a commit that referenced this pull request Jan 15, 2024
@Taragolis @ephraimbuddy
Fix check on subclass for typing.Union in _infer_multiple_outputs…
bf93554 
… for Python 3.10+ (#36728)

* Fix check on subclass for `typing.Union` in `_infer_multiple_outputs` for Python 3.10+

* Limit PEP 604 test by Python 3.10

(cherry picked from commit f1d8297)
@ephraimbuddy ephraimbuddy mentioned this pull request Jan 16, 2024
Closed
68 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@josh-fell josh-fell josh-fell approved these changes

@eladkal eladkal eladkal approved these changes

@potiuk potiuk Awaiting requested review from potiuk

@hussein-awala hussein-awala Awaiting requested review from hussein-awala

@vincbeck vincbeck Awaiting requested review from vincbeck

Assignees
No one assigned
Labels
full tests needed We need to run full set of tests for this PR to merge type:bug-fix Changelog: Bug Fixes
Projects
None yet
Milestone
Airflow 2.8.1
Development

Successfully merging this pull request may close these issues.
3 participants
@Taragolis @eladkal @josh-fell